feat: name leftover complete-case coverage (v2.12.17) - #518
Conversation
|
Warning Review limit reachedNext included review available in 8 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (21)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Integration repair at |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
The failed frontend gate exposed a static coverage caption as a second live Exact head |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Fixed the complete-case edge contract on exact head 3117823: when no complete-case rectangle exists, map item/post coverage is now 0/0 and leftover pairs remain unavailable instead of inventing residual-to-center distances as Gabriel map distances. Rank-0 complete rectangles still keep the ADR-defined stable zero-distance pair. All four review threads are resolved. Exact local validation: 127 backend/schema/API tests passed with 6 integration skips; git diff --check passed. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
* feat: persist leftover observed Y and expected E (v2.12.20) After seed, leftover closest/farthest pairs sit above the member list with observed Y and expected E[Y|θ, item] next to leftover-map distance d. Click opens that post. Residual stays R = Y − E (Jeon et al., 2021 eq. 3; Gabriel 1971; ADR 0170). Never invent a leftover score or theta. Independent of leftover persist-map (#481), criterion landing (#485), complete-case coverage (#518), leftover-map axis share (#519), comparison-strip leftover pairs (#521), and two-axis distance (#522). Issues #79 and #87 stay open. * Repair inherited login build boundary * Move leftover evidence contract to ADR 0177 * fix: show leftover Y and E after stacking onto the workspace branch Merge #490 without force-push. Keep ADR 0049 next-action copy and name observed Y and expected E on leftover pair buttons. Replay migration 0177 on existing volumes. * fix(leftover-observed-expected): wire missing migration and translations Two genuine gaps in this PR's own diff, unrelated to any merge conflict: 1. backend/tests/test_api.py and tests/test_schema.py never applied migration 0177_report_leftover_observed_expected.sql in their test DB fixtures, even though report_ingestion.py already inserts into the observed_response/expected_response columns it adds. Every period-report test that touches leftover pairs failed with UndefinedColumnError. 2. frontend/src/i18n.ts never got the ko/zh/ja/vi translations for the two leftover-map-rank action strings that frontend/src/leftoverMapRank.ts (added downstream, in the PR that bases on this one) references — t()/tf() fall back to the raw English key for every locale, failing the i18n test suite in the whole downstream chain. Verified: 1035 Python tests pass, 362 frontend tests pass, build/lint clean.
* feat: persist leftover observed Y and expected E (v2.12.20) After seed, leftover closest/farthest pairs sit above the member list with observed Y and expected E[Y|θ, item] next to leftover-map distance d. Click opens that post. Residual stays R = Y − E (Jeon et al., 2021 eq. 3; Gabriel 1971; ADR 0170). Never invent a leftover score or theta. Independent of leftover persist-map (#481), criterion landing (#485), complete-case coverage (#518), leftover-map axis share (#519), comparison-strip leftover pairs (#521), and two-axis distance (#522). Issues #79 and #87 stay open. * Repair inherited login build boundary * Move leftover evidence contract to ADR 0177 * feat: name leftover-map rank on leftover pairs (v2.12.21) After seed, leftover closest/farthest pairs sit above the member list with leftover-map rank next to leftover-map distance d. Click opens that post. Rank is the count of Gabriel singular values above the floor (Jeon et al., 2021 eq. 3; Gabriel 1971; ADR 0171). Rank 0 names no leftover structure. Never invent a leftover score or theta. Independent of leftover persist-map (#481), criterion landing (#485), complete-case coverage (#518), leftover-map axis share (#519), comparison-strip leftover pairs (#521), two-axis distance (#522), and observed Y / expected E (#527). Issues #79 and #87 stay open. * Move leftover-map rank contract to ADR 0172 ADR 0171 is already used by analysis-run status same clock (#524). Keep leftover-map rank independent: ADR 0172 and migration 0172. Shipped 0001 / 0012 stay untouched. * fix: restore frontend build on leftover-map PR Drop unused OIDC import and unauthenticated AdminPanel that fail tsc (TS6192 / TS2322). AdminPanel stays authenticated-only. * fix: show leftover Y and E after stacking onto the workspace branch Merge #490 without force-push. Keep ADR 0049 next-action copy and name observed Y and expected E on leftover pair buttons. Replay migration 0177 on existing volumes. * fix: keep leftover-map rank after stacking onto leftover Y/E Merge stacked #527 without force-push. Persist leftover_map_rank with observed Y and expected E, and name rank on leftover pair buttons.
leftover_map_coverage_from_residual accepted cells where residual is finite while leftover_map_from_residual also requires finite expected. Today expected is always finite at observed cells so counts agree, but the two masks could drift apart and make map_post_count over-count posts the pair map excluded. Reuse the identical three-clause mask; all 15 leftover-pairs tests pass (devin review thread).
| def leftover_map_coverage_for_fit( | ||
| post_ids: list[str], | ||
| item_codes: tuple[str, ...], | ||
| matrix: np.ndarray, | ||
| model: str, | ||
| theta: np.ndarray, | ||
| fit: PolytomousFit, | ||
| ) -> LeftoverMapCoverage: | ||
| """Complete-case leftover-map coverage from the fitted main effects.""" | ||
| probs = _category_probabilities(model, theta, fit) | ||
| expected = expected_category_matrix(matrix, probs) | ||
| return leftover_pairs_from_residual(post_ids, item_codes, matrix, expected) | ||
| return leftover_map_coverage_from_residual(post_ids, item_codes, matrix, expected) |
There was a problem hiding this comment.
📝 Info: Coverage recomputes probabilities already computed for pairs
leftover_map_for_fit and leftover_map_coverage_for_fit each independently recompute category probabilities and the expected matrix (lineageweave/period_report.py:258-296). Inputs are identical and deterministic, so masks agree, but the probability/expected computation runs twice per report. Negligible for rubric-sized matrices; a shared expected matrix would remove the duplication.
Was this helpful? React with 👍 or 👎 to provide feedback.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
# Conflicts: # AGENTS.md # ARCHITECTURE.md # backend/app/report_ingestion.py # backend/tests/test_api.py # frontend/src/App.test.tsx # frontend/src/App.tsx # frontend/src/api.ts # lineageweave/leftover_pairs.py # lineageweave/period_report.py # scripts/seed_demo_data.py # tests/test_leftover_pairs.py # tests/test_period_report.py # tests/test_schema.py
| -- 0001 still upgrades. Incomplete rows stay excluded; missing cells | ||
| -- are never stored as zero. | ||
|
|
||
| create table if not exists report_leftover_map_coverage ( |
There was a problem hiding this comment.
📝 Info: New migration numbered below an existing one
0168_report_leftover_map_coverage.sql is numbered below the already-shipped 0169_report_leftover_map_axis.sql. migrate.sh globs all *.sql and reapplies them idempotently with CREATE IF NOT EXISTS, so replay works today. A future ledger-based runner that tracks a last-applied number would skip 0168 on volumes already at 0169.
Was this helpful? React with 👍 or 👎 to provide feedback.
# Conflicts: # AGENTS.md # ARCHITECTURE.md # backend/tests/test_api.py # lineageweave/leftover_pairs.py # scripts/seed_demo_data.py # tests/test_leftover_pairs.py # tests/test_schema.py
There was a problem hiding this comment.
🟡 Docstring describes removed fallback pairs
leftover_pairs_from_residual no longer emits fallback pairs; with no complete-case rectangle it now returns empty. The docstring still states 'Fallback pairs (no complete-case map) omit unexplained leftover rather than fabricating one', describing behavior that was deleted.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| cur.execute((migrations / "0168_report_leftover_map_coverage.sql").read_text()) | ||
| cur.execute((migrations / "0169_report_leftover_map_axis.sql").read_text()) |
There was a problem hiding this comment.
🔍 Seed now applies the axis migration it depends on
scripts/seed_demo_data.py:123-124 adds both the new coverage migration (0168) and the axis migration (0169). report_leftover_map_axis is created only by 0169, not by 0001, yet _persist_seed_period_report already inserts into that table. Adding the 0169 line closes a gap where the seed inserted into a table its migration list never created.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Cycle comment: leftover increment still waits for independent APPROVE + exact-head required success. Author must not self-approve. Do not squash-merge without that APPROVE. Recut leftover-map length onto protected main as independent v2.12.31 / ADR 0187 (0183–0186 claimed by open leftover share/reconstruction PRs). Do not mix leftover stacks. Issues #79 and #87 stay open. |
| @@ -0,0 +1,74 @@ | |||
| # ADR 0168 — Name leftover complete-case coverage | |||
There was a problem hiding this comment.
🟡 Duplicate ADR number 0168
The new 0168-leftover-map-complete-case-coverage.md reuses a number already held by docs/adr/0168-ontology-provenance-explorer.md. Existing citations (AGENTS.md, CHANGELOG.md, ARCHITECTURE.md) mean the ontology explorer, while the new ones (AGENTS.md, 0048-persist-lsirm-leftover-pairs.md, 0049-leftover-pair-report-ui.md) mean leftover coverage. Every reference to ADR 0168 is now ambiguous.
Prompt for agents
The new ADR file docs/adr/0168-leftover-map-complete-case-coverage.md reuses ADR number 0168, which is already assigned to docs/adr/0168-ontology-provenance-explorer.md. Renumber the new leftover-map complete-case coverage ADR to an unused number, rename the file accordingly, and update every reference to it: the title line inside the file, AGENTS.md (the Tests section coverage paragraph), ARCHITECTURE.md (Phase 6e coverage paragraph), CHANGELOG.md and CHANGELOG.d entry, docs/adr/0048-persist-lsirm-leftover-pairs.md, docs/adr/0049-leftover-pair-report-ui.md, and any code comments/docstrings citing ADR 0168 for coverage (e.g. lineageweave/leftover_pairs.py, lineageweave/period_report.py). Leave the existing ADR 0168 references that mean the ontology explorer (AGENTS.md:15, CHANGELOG.md:23, ARCHITECTURE.md:82) untouched.
Was this helpful? React with 👍 or 👎 to provide feedback.
main independently claimed ADR 0168 for the leftover-map complete-case coverage change (#518) while this branch's own ADR 0168 (ontology/provenance explorer) merged in cleanly as a separate file, since git diffs by path and both files kept distinct names. Renumber the ontology/provenance explorer ADR to 0184 (next free number after main's 0183) and update every cross-reference (CHANGELOG, AGENTS.md, ARCHITECTURE.md, the 0175 migration comment, ontology_neighborhood.py/ test/ingestion docstrings, ADR 0124's predecessor link, and the ADR README index). Main's own ADR 0168 file and its references are untouched.
* feat(ui): use four Korean analyst GNB destinations Replace BuyerNav with WorkspaceNav. Analyst chrome is 게시판, 고객 마스터, 달력, and Ask Agent. Admin stays a non-GNB destination. Unwired CalendarWeave consume fail-closes with 이 범위의 일정을 아직 받을 수 없습니다 (ADR 0183). Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com> * fix(docs): resolve ADR-0168 number collision from main merge main independently claimed ADR 0168 for the leftover-map complete-case coverage change (#518) while this branch's own ADR 0168 (ontology/provenance explorer) merged in cleanly as a separate file, since git diffs by path and both files kept distinct names. Renumber the ontology/provenance explorer ADR to 0184 (next free number after main's 0183) and update every cross-reference (CHANGELOG, AGENTS.md, ARCHITECTURE.md, the 0175 migration comment, ontology_neighborhood.py/ test/ingestion docstrings, ADR 0124's predecessor link, and the ADR README index). Main's own ADR 0168 file and its references are untouched. --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
origin/main independently landed two unrelated ADRs under number 0168 (#349's ontology/provenance explorer and #518's leftover-map complete-case coverage), which merged into main days apart without a uniqueness check catching the collision. tests/test_documentation_hygiene.py fails on main by itself, independent of this PR. Renumber the ontology/provenance explorer ADR to 0184 (smaller cross-reference footprint, and its topic has no same-numbered migration file, unlike the leftover-map ADR which shares its number with migrations/0168_report_leftover_map_coverage.sql). Update every prose and link reference across docs, code comments, and tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B5M79L945DMyMs3sg5yJ14
* test(calendar): define clean Naruon projection contract * test(http): require bounded JSON response reads * fix(http): bound admitted JSON response bodies * feat(calendar): add bounded Naruon read projection * test(calendar): cover transport and numeric hardening * docs(contract): add strict Naruon calendar schema * feat(calendar): export Naruon projection package API * docs(adr): correct pseudo-CalDAV product language * docs(adr): record clean Naruon calendar authority boundary * docs(doctoring): trace calendar contract standards * docs(changelog): add clean calendar projection fragment * test(http): cover bounded response edge branches * test(calendar): cover RFC3339 defensive branches * docs(plan): add calendar contract TDD plan * test(calendar): reject surrounding contract whitespace * fix(calendar): reject silent contract string normalization * docs(contract): align exact string whitespace rules * test(http): require exact JSON response media type * fix(http): validate exact response media types * fix(calendar): require exact provider response media type * test(calendar): assert response media-type enforcement * docs(contract): add calendar consumer conformance fixture * test(calendar): pin consumer conformance fixture digest * fix: contain raw provider failures * fix: preserve login return URL and guard admin token * fix: degrade malformed chat provider replies (#379) * fix: degrade malformed chat provider replies * test: seed global ask malformed provider regression * fix(docs,worker): free ADR 0123 collision and stop mislabeling empty-body failures Renumber the Naruon calendar projection ADR to 0143 -- it collided with docs/adr/0123-provider-error-boundary.md, which four other open PRs already carry for an unrelated decision and which CI's ADR-uniqueness test would reject on merge. Add ADR 0123 itself for the provider-error-hiding policy this PR implements in auth.py, analysis_run_start.py, and chat_completion_content, since that policy previously shipped without a decision record. Stop process_post_content_job from blaming an empty source post body on a "provider operation failure" that retrying cannot fix; report it with its own failure code and an accurate detail instead of falling into the generic provider-failure catch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B5M79L945DMyMs3sg5yJ14 * docs(adr): assign Naruon calendar ADR 0145 * Revert "fix: preserve login return URL and guard admin token" This reverts commit b566c6d. * test: prove bounded calendar transport edges * Reapply "fix: preserve login return URL and guard admin token" This reverts commit dd463ed. * fix(worker): restore diagnostic log for empty-body ingestion skip Moving the empty-body guard out of the try/except in process_post_content_job (26f25ac) stopped it from raising, so the _logger.exception call that used to record the skip never runs. Operators tailing logs lost visibility into why a post-content job requeued with post_content_source_body_missing; the failure was only discoverable via a DB query. Log a warning at the guard itself, matching the _LOGGER.warning(..., extra={...}) pattern already used in post_content_persistence.py. Addresses Devin Review comment on PR #355 (post_content_worker.py:234). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XKohNQEbNH1mpNEkvsuhKv * fix(docs): renumber Naruon calendar ADR from 0145 to 0168 Both this PR and origin/main independently claimed ADR number 0145 (psychometric-channel-weight-estimation on main). Renumber this PR's ADR to the next free number to resolve the collision caught by tests/test_documentation_hygiene.py. * fix(docs): free pre-existing ADR 0168 collision on main origin/main independently landed two unrelated ADRs under number 0168 (#349's ontology/provenance explorer and #518's leftover-map complete-case coverage), which merged into main days apart without a uniqueness check catching the collision. tests/test_documentation_hygiene.py fails on main by itself, independent of this PR. Renumber the ontology/provenance explorer ADR to 0184 (smaller cross-reference footprint, and its topic has no same-numbered migration file, unlike the leftover-map ADR which shares its number with migrations/0168_report_leftover_map_coverage.sql). Update every prose and link reference across docs, code comments, and tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B5M79L945DMyMs3sg5yJ14 * fix(calendar): reconcile current main review findings * fix(ingestion): fail empty bodies without retry * fix(adr): allocate Naruon decision 0203 --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: seonghobae <seonghobae@users.noreply.github.com>
…588) * feat: name leftover-map reconstruction on leftover pairs (v2.12.31) Persist two-axis Gabriel reconstruction R̂ = ξ_{1:2} · ζ_{1:2} on period leftover pairs so landed unexplained leftover U = R − R̂ stays auditable as U + R̂ = R (ADR 0183). Reconstruction is the same internal two-axis inner product already used for U; do not substitute the centered R̃ reconstruction claimed by leftover stack #564. After make seed, closest and farthest leftover pairs sit above the member list with signed R̂ next to leftover-map distance d; click opens that post. Missing or non-finite reconstruction omits the badge rather than inventing a leftover score. Signed reconstruction is stored, never clamped. Complete-case coverage (ADR 0168) stays independent. Independent of leftover stacks #485, #518 (landed), #521, #537, #539, #563, #564, and #579. * fix(adr): allocate reconstruction decision 0201 * fix: restore R̂-persistence docs and finish ADR 0183→0201 renumbering The prior merge of origin/main into this branch (91d8e4f) resolved the AGENTS.md/ARCHITECTURE.md/CLAUDE.md conflicts by dropping this PR's own leftover-map reconstruction documentation: - AGENTS.md reverted to "Two-axis reconstruction R̂ is not persisted", contradicting the shipped migration/ADR 0201 that persists it, and dropped ADR 0201 from the governing-ADR list. - ARCHITECTURE.md kept a redundant "0183 / 0201" pair (0183 is this repo's real, unrelated ADR 0183 "GNB four Korean chrome"; the stale 0183 leftover-map reference should have been renumbered to 0201, not kept alongside it). - CLAUDE.md's "Where the rest lives" pointer never got the ADR 0201 cross-reference added. Also finishes the ADR-number renumbering the PR's own history had started (docs/adr/0201-leftover-map-reconstruction.md is the actual ADR; ADR 0183 already belongs to a different, already-landed decision on main): fixes remaining stale "ADR 0183" citations in docs/adr/0003, the 2.12.31 CHANGELOG.d fragment, and the migration 0183 SQL header comment, all of which should cite ADR 0201. docs/adr/0049's own conflict resolution (0158 + 0201 amendments) was already correct and is unchanged. * docs: reconcile ADR 0182 reconstruction context * test(leftover): cover reconstruction candidate contract * fix(leftover): prove rank-zero reconstruction identity --------- Co-authored-by: seonghobae <seonghobae@users.noreply.github.com>
1 participant
Independent leftover-map honesty slice on protected
main@ ef6f5a5 (v2.12.6). Does not mix into #481 (map coords), #485 (criterion landing), #502 (residual R; ontoworktree-fix-frontend-build-break), or #426.Buyer gap
The leftover map already drops incomplete rows rather than treating missing cells as 0 (Gabriel 1971; Jeon et al. 2021 eq. 3; ADR 0048). The period report did not tell the buyer how many scored posts entered that factorization. A sparse post can be scored, excluded from the map, and never named as closest/farthest — silently.
What this ships (ADR 0165)
leftover_map_coverage_from_residualnamesmap_post_countofscored_post_count(incomplete rows excluded, never zero).report_leftover_map_coverage(3NF, two-or-more-wordsnake_case).GET /api/reports/{grouping}/{period}returnsleftover_map_coverage.Not this PR
Coordinates, residual-row disclosure, pair click → criterion landing, two-axis distance, Y/E.
Review
Independent exact-head APPROVE required. Do not self-approve. #79 and #87 stay open.
Tests
tests/test_leftover_pairs.pyandtests/test_migration_replay.py(6 passed locally). Frontend caption assertion added. Live Postgres/API tests self-skip without the stack.